-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
replace firebase authentication with jwt #3
base: master
Are you sure you want to change the base?
Conversation
@@ -38,11 +38,13 @@ | |||
"supertest": "^6.1.6" | |||
}, | |||
"dependencies": { | |||
"basic-server-backend": "file:", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is that supposed to do? can you please remove it?
"basic-server-backend": "file:", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes,It is removed now please check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it still hasn't
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are a few changes to be made, if you want you can start from here
it will give you a head start, it's 90% complete
@@ -1,26 +1,55 @@ | |||
const { auth } = require("../firebase"); | |||
const jwt = require("jsonwebtoken"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should make use of the JWT/index.js
instead of re doing the same flow again
// Function to generate JWT token | ||
const generateToken = (payload) => { | ||
return jwt.sign(payload, config.get("jwt_secret"), { expiresIn: "1h" }); | ||
}; | ||
|
||
// Function to verify JWT token | ||
const verifyToken = (token) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should make use of the JWT/index.js
instead of
return jwt.verify(token, config.get("jwt_secret")); | ||
} catch (err) { | ||
console.error("Token verification failed:", err); | ||
return null; | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should make use of the JWT/index.js
instead of
// Example usage in authenticateUser function | ||
const authenticateUser = (req, res) => { | ||
const { username, password } = req.body; | ||
|
||
// Replace this with your user authentication logic | ||
const user = authenticateWithDatabase(username, password); | ||
|
||
if (user) { | ||
const token = generateToken({ id: user.id, username: user.username }); | ||
res.json({ token }); | ||
} else { | ||
res.status(401).json({ message: "Authentication failed" }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these example should be part of the README or docs (which doesn't exist right now) so we can have it in the user controller instead
// Example middleware to protect routes | ||
const authenticateMiddleware = (req, res, next) => { | ||
const token = req.headers["authorization"]; | ||
|
||
if (!token) { | ||
return res.status(403).json({ message: "No token provided" }); | ||
} | ||
|
||
const decoded = verifyToken(token); | ||
|
||
if (!decoded) { | ||
return res.status(401).json({ message: "Failed to authenticate token" }); | ||
} | ||
|
||
req.user = decoded; | ||
next(); | ||
}; | ||
|
||
module.exports = { | ||
authenticateUser, | ||
authenticateMiddleware, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for the contribution! Even though the dependency have been removed from firebase there are a few more changes still before we merge it!
Although based on your efforts, I have marked it approved for hacktoberfest.
if you're still looking to contribute anywhere, here's another project where your contribution are of need waltzes
No description provided.